-
Notifications
You must be signed in to change notification settings - Fork 587
Bottomhalf example cleanup and doc update #338
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
261623c
to
f7b7075
Compare
A common way to avoid blocking the interrupt for a significant duration | ||
is to defer the time-consuming part to a workqueue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve the writing for rationale.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you suggesting that the rationale behind the change needs more explanation, or
Is the issue with the tone and professionalism of the writing itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you suggesting that the rationale behind the change needs more explanation?
I meant that you can consolidate BH descriptions, such as the removal of tasklet.
f7b7075
to
a41a967
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
lkmpg.tex
Outdated
such as memory management issues and unpredictable latencies. | ||
Instead, they recommend more robust mechanisms like workqueues or softirqs. | ||
To address tasklet shortcomings, Linux contributors introduced the BH workqueue, | ||
activated with the WQ_BH flag. This workqueue retains critical features, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add \cpp
to WQ_BH
.
Split the sentence starting with "This workqueue" into a newline.
lkmpg.tex
Outdated
Instead, they recommend more robust mechanisms like workqueues or softirqs. | ||
To address tasklet shortcomings, Linux contributors introduced the BH workqueue, | ||
activated with the WQ_BH flag. This workqueue retains critical features, | ||
such as execution in atomic (softirq) context on the same CPU and the inability to sleep. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Add \textbf{softirq}
for consistency
Commit 3682d9e ("Move bottom-half to workqueue for safe sleep") migrated the example code away from tasklets to workqueues. However, the compatibility macro DECLARE_TASKLET_OLD was left behind even though tasklets are no longer used. Remove the unused macro for consistency and clarity.
Since commit 3682d9e ("Move bottom-half to workqueue for safe sleep") switched the implementation to workqueues, update the description accordingly to avoid confusion and reflect the modern approach to bottom-half handling. Explain the issues with tasklets and introduce the BH workqueue as the new, reliable alternative.
a41a967
to
6807fb9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Thank @eleanorLYJ for contributing! |
There are two commits:
Keep the code and docs consistent, avoiding confusion.